Skip to content

Conversation

@ndrs-pst
Copy link
Contributor

Since the _impl naming convention is intended for internal use only, renaming these functions to the shell_fprintf_xxx variant is more suitable for calls outside the module:

  • shell_info_impl to shell_fprintf_info
  • shell_print_impl to shell_fprintf_normal
  • shell_warn_impl to shell_fprintf_warn
  • shell_error_impl to shell_fprintf_error

This change is related to PR #77165.

@ndrs-pst
Copy link
Contributor Author

👀 @jakub-uC just a ping to see if you might have a moment to review.

jakub-uC
jakub-uC previously approved these changes Aug 28, 2024
@jakub-uC
Copy link
Contributor

What about changing shell_fprintf_impl to shell_fprintf? As it was couple years ago? ;-)

@ndrs-pst
Copy link
Contributor Author

What about changing shell_fprintf_impl to shell_fprintf? As it was couple years ago? ;-)

OK, wait a moment. This also makes me curious why we use a wrapper macro when we can call the exact function. 😃

jakub-uC
jakub-uC previously approved these changes Aug 28, 2024
pdgendt
pdgendt previously approved these changes Aug 28, 2024
jukkar
jukkar previously approved these changes Aug 28, 2024
@ndrs-pst ndrs-pst force-pushed the pr_shell_rename_shell_xxx_impl branch from 9cd02d6 to 5927eeb Compare August 29, 2024 01:35
@ndrs-pst
Copy link
Contributor Author

@jakub-uC Seem this piece of test code block the CI.
Which seem to require to use shell_printf as macro.

#define CUSTOM_SHELL_PREFIX "[CUSTOM_PREFIX]"
#undef shell_fprintf
#define shell_fprintf(sh, color, fmt, ...) \
shell_fprintf_impl(sh, color, CUSTOM_SHELL_PREFIX fmt, ##__VA_ARGS__)

@ndrs-pst
Copy link
Contributor Author

ndrs-pst commented Aug 29, 2024

According to this PR #71290 I think my change should be reverted. 😅


Drop the second commit, as I can't figure out the workaround without affecting the changes in PR #71290.

Since the `_impl` naming convention is intended for internal use only,
renaming these functions to the `shell_fprintf_xxx` variant is
more suitable for calls outside the module:
- `shell_info_impl` to `shell_fprintf_info`
- `shell_print_impl` to `shell_fprintf_normal`
- `shell_warn_impl` to `shell_fprintf_warn`
- `shell_error_impl` to `shell_fprintf_error`

Signed-off-by: Pisit Sawangvonganan <[email protected]>
@ndrs-pst ndrs-pst dismissed stale reviews from jakub-uC, pdgendt, and jukkar via 8203656 September 8, 2024 15:35
@ndrs-pst ndrs-pst force-pushed the pr_shell_rename_shell_xxx_impl branch from 5927eeb to 8203656 Compare September 8, 2024 15:35
@nashif nashif merged commit d207edb into zephyrproject-rtos:main Sep 9, 2024
ndrs-pst added a commit to DDC-NDRS/zephyr_rtos that referenced this pull request Sep 10, 2024
…intf`

Due to the introduction of `shell_xxx_impl` wrapper functions in
PR zephyrproject-rtos#75340 and rename to `shell_fprintf_xxx` in PR zephyrproject-rtos#77192 we can minimize
caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
@ndrs-pst ndrs-pst deleted the pr_shell_rename_shell_xxx_impl branch September 10, 2024 02:57
carlescufi pushed a commit that referenced this pull request Sep 10, 2024
…intf`

Due to the introduction of `shell_xxx_impl` wrapper functions in
PR #75340 and rename to `shell_fprintf_xxx` in PR #77192 we can minimize
caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
ndrs-pst added a commit to DDC-NDRS/zephyr_rtos that referenced this pull request Sep 13, 2024
…intf`

Due to the introduction of `shell_fprintf_normal` in PR zephyrproject-rtos#77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
nashif pushed a commit that referenced this pull request Sep 17, 2024
…intf`

Due to the introduction of `shell_fprintf_normal` in PR #77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
ndrs-pst added a commit to DDC-NDRS/zephyr_rtos that referenced this pull request Jan 22, 2025
…intf`

Due to the introduction of `shell_fprintf_normal` in PR zephyrproject-rtos#77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
kartben pushed a commit that referenced this pull request Mar 24, 2025
…intf`

Due to the introduction of `shell_fprintf_normal` in PR #77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
jeppenodgaard pushed a commit to jeppenodgaard/zephyr that referenced this pull request Mar 25, 2025
…intf`

Due to the introduction of `shell_fprintf_normal` in PR zephyrproject-rtos#77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
KwsBaer pushed a commit to kwsgmbh/zephyr that referenced this pull request Apr 3, 2025
…intf`

Due to the introduction of `shell_fprintf_normal` in PR zephyrproject-rtos#77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
csarip pushed a commit to csarip/zephyr that referenced this pull request Jun 2, 2025
…intf`

Due to the introduction of `shell_fprintf_normal` in PR zephyrproject-rtos#77192, we can
minimize caller overhead by eliminating direct `color` parameter passing.

Signed-off-by: Pisit Sawangvonganan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants